-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix(web-react): Replace usage of html-react-parser
in CommonJS files
#1289
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* @see: https://www.npmjs.com/package/html-react-parser#usage * according to docs the `html-react-parser` should be required in CommonJS as `require('html-react-parser').default` but our build process creates the files without the `.default` export * this replacement post process is the workaround to fix this issue
✅ Deploy Preview for spirit-design-system-demo canceled.
|
✅ Deploy Preview for spirit-design-system-storybook canceled.
|
✅ Deploy Preview for spirit-design-system-validations canceled.
|
✅ Deploy Preview for spirit-design-system-react canceled.
|
Reference to the issue where it can be solved: remarkablemark/html-react-parser#1329 (comment) |
pavelklibani
approved these changes
Feb 23, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
html-react-parser
should be required in CommonJSas
require('html-react-parser').default
but our build process creates the fileswithout the
.default
exportAdditional context
It seems like you're dealing with a common issue that arises when using a mix of ES modules and CommonJS modules. The problem is that when you're using a bundler like Webpack or Rollup, they don't always know how to correctly import a CommonJS module into an ES module.
One way to solve this is to manually specify the import in your code. Instead of:
You can use:
This explicitly tells the bundler to use the default export from the html-react-parser module.
If you're using TypeScript and you have enabled the esModuleInterop option in your tsconfig.json, you should be able to use the default import syntax:
And TypeScript will automatically compile it to:
If none of these solutions work, you might need to configure your bundler to handle this specific case. The exact solution will depend on the bundler you're using. For example, in Webpack, you can use the alias configuration option to specify how a module should be imported. In Rollup, you can use the namedExports option of the commonjs plugin.
As of Rollup Plugin CommonJS v9.3.0, the namedExports option has been removed because Rollup now handles this automatically.
If you're still having issues, it might be due to how the html-react-parser package is exported. If the package's main file doesn't have a default export, you might need to import it like this:
And then use it like this:
If this doesn't work, you can try using the rollup-plugin-replace to replace the import statement during the build process. Here's an example of how to do this:
This will replace all instances of require('html-react-parser') with require('html-react-parser').default in your bundled code. Please note that this is a workaround and might not work in all cases.
In this situation, you might need to use a dynamic require statement in your code. This is not ideal because it can make your code harder to analyze and optimize for bundlers and linters, but it can be a workaround for this specific issue.
Here's how you can do it:
This will ensure that the default export from html-react-parser is used, regardless of how your code is transpiled.
Please note that this is a workaround and might not work in all cases. It's generally better to use ES modules syntax (import/export) in your code and let your bundler handle the conversion to CommonJS if necessary. However, this might not be possible if a package is not correctly exported as an ES module.
I see, you're generating both ESM and CJS versions from the same source using Rollup. In this case, you might want to consider using the rollup-plugin-commonjs-alternate plugin. This plugin is a fork of the original rollup-plugin-commonjs and adds support for a namedExports option, which can be used to manually specify named exports for specific modules.
Here's how you can use it:
This will tell Rollup to treat the default export from html-react-parser as a named export, which should allow you to import it correctly in your code.
Please note that this is a workaround and might not work in all cases. It's generally better to use ES modules syntax (import/export) in your code and let your bundler handle the conversion to CommonJS if necessary. However, this might not be possible if a package is not correctly exported as an ES module.
Yes, you can certainly do that as a workaround. You can use a script to post-process the transpiled files and replace the require('html-react-parser') with require('html-react-parser').default.
Issue reference
Before submitting the PR, please make sure you do the following
fixes #123
).